-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(node): added wp cli command - process pending requests #67
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great @jaredrethman! Nice work 🎉
I left two small comments regarding output, then I think this is good to go.
pr feedback adds better output when no requests are processed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick turnaround! I have one small nit, but since the current messaging works and I don't wanna hold this up, gonna go ahead and approve.
|
||
if ( ! empty( $unprocessed_request_ids ) ) { | ||
WP_CLI::error( "The following requests could not be processed:\n" . wp_json_encode( $unprocessed_request_ids ) ); | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit. Feel free to ignore. I think we were okay with the warning here. For large batches of events where some fail and some succeed, I would expect to see information about what failed and what succeeded.
Thinking about this some more, something like the following would be ideal IMO:
success - all events processed
warning - some events processed, some events not processed
error - no events processed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some solid suggestions there @chickenn00dle! I have implemented your suggestions, capture last error (i.e. the one that caused the error to be captured) and added some additional comments/docs etc 36049d4, all-in-all your suggestion adds a lot of value to the command, thank you 🤜 🤛
Success:
Warning: (hard to test)
Errors:
total vs. success vs. failure
# [1.3.0-alpha.1](v1.2.0...v1.3.0-alpha.1) (2024-02-23) ### Bug Fixes * memberships sync ([#63](#63)) ([0a54f1d](0a54f1d)) ### Features * add hub bookmarks to nodes ([#56](#56)) ([54700a0](54700a0)) * add option to manually sync users ([#53](#53)) ([3ec1b19](3ec1b19)) * display membership plans from the network ([#59](#59)) ([6fa01fd](6fa01fd)) * enhance network subscriptions and orders search ([#55](#55)) ([bcb0615](bcb0615)) * Node connection using a link ([#58](#58)) ([721f8b2](721f8b2)) * **wp-cli:** add command to backfill events ([#51](#51)) ([13d2498](13d2498)) * **wp-cli:** add process pending webhook requests command ([#67](#67)) ([7dbd8dc](7dbd8dc)) * **wp-cli:** sync all events from the Hub ([#65](#65)) ([dc595ca](dc595ca))
🎉 This PR is included in version 1.3.0-alpha.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [1.3.0](v1.2.0...v1.3.0) (2024-02-28) ### Bug Fixes * backfill duplicate handling; woo links in wp-admin-bar ([#71](#71), [#72](#72)) ([bbce13b](bbce13b)) * cli commands ([#73](#73)) ([ff563ac](ff563ac)) * memberships sync ([#63](#63)) ([0a54f1d](0a54f1d)) * missing woocommerce-memberships handling ([76dbdf7](76dbdf7)) ### Features * add hub bookmarks to nodes ([#56](#56)) ([54700a0](54700a0)) * add option to manually sync users ([#53](#53)) ([3ec1b19](3ec1b19)) * display membership plans from the network ([#59](#59)) ([6fa01fd](6fa01fd)) * enhance network subscriptions and orders search ([#55](#55)) ([bcb0615](bcb0615)) * Node connection using a link ([#58](#58)) ([721f8b2](721f8b2)) * **wp-cli:** add command to backfill events ([#51](#51)) ([13d2498](13d2498)) * **wp-cli:** add process pending webhook requests command ([#67](#67)) ([7dbd8dc](7dbd8dc)) * **wp-cli:** sync all events from the Hub ([#65](#65)) ([dc595ca](dc595ca))
🎉 This PR is included in version 1.3.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
All Submissions:
Changes proposed in this Pull Request:
This PR introduces a WP CLI command for processing
np_webhook_request
s with post-meta status inpending
state.How to test the changes in this Pull Request:
wp newspack-network process-webhooks
to process and delete pending requestsOther information: